feat: feat: add to_time, to_local_time, to_date functions#1387
feat: feat: add to_time, to_local_time, to_date functions#1387mesejo wants to merge 1 commit intoapache:mainfrom
Conversation
mesejo
force-pushed
the
feat/expose_to_time_function
branch
2 times, most recently
from
1297aae to
5c639b5
Compare
mesejo
changed the title
mesejo
force-pushed
the
feat/expose_to_time_function
branch
3 times, most recently
from
18b3258 to
2c76187
Compare
Additionally fix conditional on formatters (since it is *args it cannot be None) Refactor name to avoid possible collision with f.
mesejo
force-pushed
the
feat/expose_to_time_function
branch
from
2c76187 to
a8d1166
Compare
| formatters = [fmt.expr for fmt in formatters] | ||
| return Expr(f.to_date(arg.expr, *formatters)) |
There was a problem hiding this comment.
to_date, to_time, and to_timestamp* each perform the same formatter-unwrapping ([fmt.expr for fmt in formatters]) and “no formatters” branching.
What do you think of extracting a tiny private helper (e.g. _unwrap_exprs(*args: Expr) -> list[PyExpr]) and/or a shared optional-format dispatch helper to reduce duplication and keep behavior aligned across functions?
| return Expr(f.now()) | ||
|
|
||
|
|
||
| def to_char(arg: Expr, format: Expr) -> Expr: |
There was a problem hiding this comment.
format shadows Python’s built-in format.
What do you think of renaming to formatter like in to_date?
| ); | ||
| expr_fn!(now); | ||
| expr_fn_vec!(to_date); | ||
| expr_fn_vec!(to_local_time); |
There was a problem hiding this comment.
to_local_time is exposed in Python as unary (def to_local_time(arg: Expr)).
Why do we need expr_fn_vec! here?
If expr_fn_vec! is intentionally required by upstream API shape, a short comment here would prevent future confusion.
| return Expr(f.current_date()) | ||
|
|
||
|
|
||
| today = current_date |
There was a problem hiding this comment.
This alias appears unrelated to the PR and is not added to __all__ or covered by tests/docs.
| "02-07-2020", | ||
| ], | ||
| type=pa.string(), | ||
| ) |
There was a problem hiding this comment.
Good positive-path coverage was added to test_temporal_functions
Consider adding some focused negative-case assertion for invalid formatter/input for the new API (e.g. malformed to_time formatter), to ensure error surfaces are stable and user-facing diagnostics remain clear.
2 participants
Which issue does this PR close?
N/A
Rationale for this change
Expose missing temporal functions
What changes are included in this PR?
The functions
to_time,to_local_time,to_date, and the corresponding tests.Fix conditional on
to_timestampsince it is args it can not beNone, see:Output
()
Add the alias
todayfor thecurrent_datefunctionAre there any user-facing changes?
Yes, new functions were added